[PATCH] src: handle NGHTTP2_ERR_FLOW_CONTROL error code

Refs: https://hackerone.com/reports/3531737
PR-URL: https://github.com/nodejs-private/node-private/pull/832
CVE-ID: CVE-2026-21714

Gbp-Pq: Topic sec
Gbp-Pq: Name 55-handle-NGHTTP2_ERR_FLOW_CONTROL-error-code.patch

diff --git a/src/node_http2.cc b/src/node_http2.cc
index ba01b6eab2ff00862a2e857bd8c3ab5ebc2747e8..9a0c264ea610285429bc980e19e48830dacef19b 100644 (file)
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -1116,8 +1116,14 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
   // The GOAWAY frame includes an error code that indicates the type of error"
   // The GOAWAY frame is already sent by nghttp2. We emit the error
   // to liberate the Http2Session to destroy.
+  //
+  // ERR_FLOW_CONTROL: A WINDOW_UPDATE on stream 0 pushed the connection-level
+  // flow control window past 2^31-1. nghttp2 sends GOAWAY internally but
+  // without propagating this error the Http2Session would never be destroyed,
+  // causing a memory leak.
   if (nghttp2_is_fatal(lib_error_code) ||
       lib_error_code == NGHTTP2_ERR_STREAM_CLOSED ||
+      lib_error_code == NGHTTP2_ERR_FLOW_CONTROL ||
       lib_error_code == NGHTTP2_ERR_PROTO) {
     Environment* env = session->env();
     Isolate* isolate = env->isolate();

diff --git a/test/parallel/test-http2-window-update-overflow.js b/test/parallel/test-http2-window-update-overflow.js
new file mode 100644 (file)
index 0000000..41488af
--- /dev/null
+++ b/test/parallel/test-http2-window-update-overflow.js
@@ -0,0 +1,84 @@
+'use strict';
+
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const http2 = require('http2');
+const net = require('net');
+
+// Regression test: a connection-level WINDOW_UPDATE that causes the flow
+// control window to exceed 2^31-1 must destroy the Http2Session (not leak it).
+//
+// nghttp2 responds with GOAWAY(FLOW_CONTROL_ERROR) internally but previously
+// Node's OnInvalidFrame callback only propagated errors for
+// NGHTTP2_ERR_STREAM_CLOSED and NGHTTP2_ERR_PROTO. The missing
+// NGHTTP2_ERR_FLOW_CONTROL case left the session unreachable after the GOAWAY,
+// causing a memory leak.
+
+const server = http2.createServer();
+
+server.on('session', common.mustCall((session) => {
+  session.on('error', common.mustCall());
+  session.on('close', common.mustCall(() => server.close()));
+}));
+
+server.listen(0, common.mustCall(() => {
+  const conn = net.connect({
+    port: server.address().port,
+    allowHalfOpen: true,
+  });
+
+  // HTTP/2 client connection preface.
+  conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n');
+
+  // Empty SETTINGS frame (9-byte header, 0-byte payload).
+  const settingsFrame = Buffer.alloc(9);
+  settingsFrame[3] = 0x04; // type: SETTINGS
+  conn.write(settingsFrame);
+
+  let inbuf = Buffer.alloc(0);
+  let state = 'settingsHeader';
+  let settingsFrameLength;
+
+  conn.on('data', (chunk) => {
+    inbuf = Buffer.concat([inbuf, chunk]);
+
+    switch (state) {
+      case 'settingsHeader':
+        if (inbuf.length < 9) return;
+        settingsFrameLength = inbuf.readUIntBE(0, 3);
+        inbuf = inbuf.slice(9);
+        state = 'readingSettings';
+        // Fallthrough
+      case 'readingSettings': {
+        if (inbuf.length < settingsFrameLength) return;
+        inbuf = inbuf.slice(settingsFrameLength);
+        state = 'done';
+
+        // ACK the server SETTINGS.
+        const ack = Buffer.alloc(9);
+        ack[3] = 0x04; // type: SETTINGS
+        ack[4] = 0x01; // flag: ACK
+        conn.write(ack);
+
+        // WINDOW_UPDATE on stream 0 (connection level) with increment 2^31-1.
+        // Default connection window is 65535, so the new total would be
+        // 65535 + 2147483647 = 2147549182 > 2^31-1, triggering
+        // NGHTTP2_ERR_FLOW_CONTROL inside nghttp2.
+        const windowUpdate = Buffer.alloc(13);
+        windowUpdate.writeUIntBE(4, 0, 3);          // length = 4
+        windowUpdate[3] = 0x08;                      // type: WINDOW_UPDATE
+        windowUpdate[4] = 0x00;                      // flags: none
+        windowUpdate.writeUIntBE(0, 5, 4);           // stream id: 0
+        windowUpdate.writeUIntBE(0x7FFFFFFF, 9, 4);  // increment: 2^31-1
+        conn.write(windowUpdate);
+      }
+    }
+  });
+
+  // The server must close the connection after sending GOAWAY.
+  conn.on('end', common.mustCall(() => conn.end()));
+  conn.on('close', common.mustCall());
+}));